Fix unbounded enumeration in postgres list#2920
Merged
Merged
Conversation
Cap both 'azmcp postgres list' database and table results at 10,000 entries to prevent OOM/perf issues on servers with very large catalogs, mirroring the existing MySQL behavior: - Add MaxRowCount = 10_000 constant in PostgresService (parity with MySQL's MaxRowCount). - ListDatabasesAsync: ORDER BY datname LIMIT @MaxResults; append the same '... (output limited to 10,000 databases ...)' sentinel row that MySQL appends when the cap is reached. - ListTablesAsync: ORDER BY table_name LIMIT @MaxResults (cap+1) and return new TableListResult(Tables, IsTruncated) so the command can surface truncation via an explicit flag. Truncation is detected via Count > cap rather than the MySQL read-then-probe pattern, which silently swallows the truncation signal when combined with a SQL LIMIT cap+1. - IPostgresService updated to return TableListResult from ListTablesAsync. - PostgresListCommandResult gains an optional 'tablesTruncated' bool. - Add unit tests for the new cap/truncation behavior and update existing PostgresListCommand tests for the new return type. - Document the cap in azmcp-commands.md and add a Bugs Fixed changelog entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
postgres list
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses unbounded enumeration in azmcp postgres list by introducing a hard cap (10,000) for database/table listing to avoid streaming extremely large result sets into memory, aligning behavior with the existing MySQL tooling patterns.
Changes:
- Add a 10,000 row cap for PostgreSQL database/table listing, with truncation signaling (sentinel row for databases;
tablesTruncatedflag for tables). - Introduce
TableListResultto return both table names and a truncation indicator from the service layer. - Add/extend tests plus doc/changelog updates to reflect the new bounded behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Postgres/src/Services/PostgresService.cs | Adds MaxRowCount and applies SQL LIMIT + truncation signaling for database/table listing. |
| tools/Azure.Mcp.Tools.Postgres/src/Services/IPostgresService.cs | Updates ListTablesAsync contract to return TableListResult. |
| tools/Azure.Mcp.Tools.Postgres/src/Services/TableListResult.cs | New result type for tables list + truncation flag. |
| tools/Azure.Mcp.Tools.Postgres/src/Commands/PostgresListCommand.cs | Surfaces table truncation via TablesTruncated in the command result. |
| tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.Tests/PostgresListCommandTests.cs | Updates tests for new service return type and validates TablesTruncated behavior. |
| tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.Tests/Services/PostgresServiceRowLimitTests.cs | New tests covering under-cap/at-cap/over-cap behaviors. |
| servers/Azure.Mcp.Server/docs/azmcp-commands.md | Documents the new caps and truncation signaling for azmcp postgres list. |
| servers/Azure.Mcp.Server/changelog-entries/1782105637872.yaml | Adds a changelog entry for the bug fix and output-shape details. |
Address alzimmermsft's PR review: - ListDatabasesAsync now returns DatabaseListResult with an IsTruncated flag instead of appending a sentinel row, and uses LIMIT cap+1 so truncation is actually detectable. - Surface DatabasesTruncated on PostgresListCommandResult. - Make PostgresService.MaxRowCount internal so tests reference it instead of duplicating the constant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A postgres list call returns servers, databases, or tables - never more than one - so a single ResultsTruncated flag is sufficient instead of separate DatabasesTruncated/TablesTruncated fields. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reconcile #472 row-limit/truncation work with main's Postgres refactor: - Adopt main's removal of subscriptionId/resourceGroup from service methods - Adopt main's parameterized @Schema filter on ListTablesAsync - Keep DatabaseListResult/TableListResult return types and cap+1 truncation - Combine @Schema filter with LIMIT @MaxResults in the tables query - Update tests for new signatures (drop sub/rg, add schema) and the parameterized-schema test to expect the added maxResults parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
alzimmermsft
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
postgres list databasesandpostgres list tablespreviously returned every row frompg_database/information_schema.tableswith no upper bound, so a server with hundreds of databases or thousands of tables would stream everything into memory with no cap and no truncation signal to the caller. This PR closes that gap by mirroring the existingmysql listrow-cap pattern.Approach (parity with MySQL):
MaxRowCount = 10_000constant inPostgresService.ListDatabasesAsync: query now usesORDER BY datname LIMIT @maxResults. When the result hits the cap, a human-readable sentinel row is appended to the list (identical wording and behavior toMySqlService.ListDatabasesAsync).ListTablesAsync: now returns a newTableListResult(List<string> Tables, bool IsTruncated)sealed record (mirrorsMySql/.../TableListResult.cs). Query usesORDER BY table_name LIMIT @maxResults + 1; truncation is detected viatables.Count > MaxRowCount, then the extra row is trimmed.PostgresListCommandsurfaces the flag viabool? TablesTruncatedonPostgresListCommandResult(4th positional param, defaultnull), matchingMySqlListCommandResultexactly.bool?+JsonIgnoreCondition.WhenWritingNullkeeps the field absent from non-truncated responses for backward compatibility.Notes:
LIMIT capvsLIMIT cap+1) because they use different truncation signals (sentinel row vs structured flag). Inline comments on both methods explain the differences.LIMIT, post-loopReadto detect overflow) but it silently fails to detect truncation once a SQLLIMITis added: the loop's exit iteration consumes the probe row. Switched toLIMIT cap+1+Count > cap, which keeps the public surface identical to MySQL.--max-resultsCLI option was added. MySQL does not expose one, so Postgres does not either (parity, simpler, backward compatible).GitHub issue number?
Fixes #472
Pre-merge Checklist
azmcp postgres listis modifiedservers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentation - N/A (tool description unchanged)README.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1- N/AToolDescriptionEvaluator- N/A (no tool description changes)consolidated-tools.json- N/A (no new/renamed tools)servers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata - N/A (no command surface or metadata change; output shape is backward compatible)servers/Azure.Mcp.Server/docs/e2eTestPrompts.md- existing prompts still applyInvoking Livetests
Copilot submitted PRs are not trustworthy by default. Users with
writeaccess to the repo need to validate the contents of this PR before leaving a comment with the text/azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.